Skip to content

Issues/224 atomic counter attribute #3809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: development
Choose a base branch
from

Conversation

irina-herciu
Copy link
Contributor

Introduce support for the [DynamoDBAtomicCounter] attribute in the DynamoDB Object Persistence Model for the AWS SDK for .NET. This feature brings parity with the Java SDK V2, which supports atomic counters via @DynamoDbAtomicCounter.

java DynamoDbAtomicCounter
Using atomic counters

Description

  • New attribute: DynamoDBAtomicCounterAttribute, to mark numeric properties for auto-increment behavior
  • Updates to DynamoDBContext and internal mapping to detect and apply counter increment behavior during save item operation
class Session {
    [DynamoDBHashKey]
    public string Id { get; set; }

    [DynamoDBAtomicCounter]
    public int RetryCount { get; set; }

    // This field starts from 100 and increments by 5 on each save
    [DynamoDBAtomicCounter(delta:5, startValue:100)] 
    public long Views { get; set; }
}

On context.SaveAsync(session), it will automatically be incremented in DynamoDB using the appropriate SET operation.

Motivation and Context

Testing

Integration test added
Unit test to be added for internal methods

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

foreach (var propertyStorage in counterPropertyStorages)
{
Primitive counter;
string versionAttributeName = propertyStorage.AttributeName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be called counterAttributeName?

string startValueName = $":{propertyStorage.AttributeName}Start";
string deltaValueName = $":{propertyStorage.AttributeName}Delta";
string counterAttributeName = Common.GetAttributeReference(propertyStorage.AttributeName);
asserts += $"{counterAttributeName} = " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious how this compares to Java V2. But if I set the StartValue to 3 and Delta to 5 when I save my first version of the object I get 8. I would expect 3 after the first save and 8 with the second save. If this is consistent with Java we need to make sure the docs on the StartValue and Delta are clear this is how it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first save will be 3 as the value in ExpressionAttributeValues is CounterStartValue-CounterDelta so that the increment can be done correctly. This is the same behavior as in Java V2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, can you add a comment by the propertyStorage.CounterStartValue - propertyStorage.CounterDelta; line that the propertyStorage.CounterDelta is being subtracted from the initial value to compensate it being added back to the starting value via the delta? Since I missed that detail reviewing the code others in the future will likely miss that as well.

await table.UpdateHelperAsync(storage.Document, table.MakeKey(storage.Document), null, cancellationToken).ConfigureAwait(false);
updateDocument = await table.UpdateHelperAsync(storage.Document, table.MakeKey(storage.Document), new UpdateItemOperationConfig
{
ReturnValues = ReturnValues.AllNewAttributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid change in behavior should we set AllNewAttributes only if there is a counter condition expression?

var updateItemOperationConfig = new UpdateItemOperationConfig
{
Expected = expectedDocument,
ReturnValues = ReturnValues.None,
ReturnValues = ReturnValues.AllNewAttributes,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid change in behavior should we set AllNewAttributes only if there is a counter condition expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AllNewAttributes is conditional now

string startValueName = $":{propertyStorage.AttributeName}Start";
string deltaValueName = $":{propertyStorage.AttributeName}Delta";
string counterAttributeName = Common.GetAttributeReference(propertyStorage.AttributeName);
asserts += $"{counterAttributeName} = " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, can you add a comment by the propertyStorage.CounterStartValue - propertyStorage.CounterDelta; line that the propertyStorage.CounterDelta is being subtracted from the initial value to compensate it being added back to the starting value via the delta? Since I missed that detail reviewing the code others in the future will likely miss that as well.

/// <returns></returns>
[TestMethod]
[TestCategory("DynamoDBv2")]
public async Task TestContext_AtomicCounterAnnotation()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a check for updating the counter value when the current read value is behind the current value in the table and confirm after the save the POCO has latest value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integration test was updated

@normj
Copy link
Member

normj commented May 21, 2025

Sent the PR through the internal build system.

@normj
Copy link
Member

normj commented May 22, 2025

I'm getting the following DDB integ test failure. Can you take a look?


[xUnit.net 00:00:06.34]     Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestContext [FAIL]
--
3720 | Failed Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestContext [3 s]
3721 | Error Message:
3722 | System.ArgumentNullException : Value cannot be null. (Parameter 'target')
3723 | Stack Trace:
3724 | at Amazon.Util.CircularReferenceTracking.Track(Object target) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Core\Amazon.Util\ReferenceTracking.cs:line 104
3725 | at Amazon.DynamoDBv2.DataModel.DynamoDBFlatConfig.OperationState.Track(Object target) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\Configs.cs:line 621
3726 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.PopulateInstance(ItemStorage storage, Object instance, DynamoDBFlatConfig flatConfig, StorageConfig storageConfig) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\ContextInternal.cs:line 443
3727 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveHelperAsync(Type valueType, Object value, DynamoDBFlatConfig flatConfig, CancellationToken cancellationToken) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\Context.cs:line 467
3728 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveHelperAsync[T](T value, DynamoDBFlatConfig flatConfig, CancellationToken cancellationToken) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\Context.cs:line 417
3729 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveAsync[T](T value, CancellationToken cancellationToken) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\_async\Context.Async.cs:line 40
3730 | at Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestHashObjects() in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\test\NetStandard\IntegrationTests\IntegrationTests\DynamoDB\DataModelTests.cs:line 508
3731 | at Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestHashObjects() in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\test\NetStandard\IntegrationTests\IntegrationTests\DynamoDB\DataModelTests.cs:line 524
3732 | at Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestContext() in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\test\NetStandard\IntegrationTests\IntegrationTests\DynamoDB\DataModelTests.cs:line 42
3733 | --- End of stack trace from previous location ---
3734 | Results File: C:\codebuild\tmp\output\src3842137447\src/aws-sdk-net/sdk\test\NetStandard\IntegrationTests\bin\Release\net8.0/TestResults\ContainerAdministrator_6997AF1862EC_2025-05-21_17_10_42.trx
3735 |  
3736 | Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: < 1 ms - IntegrationTests.dll (net8.0

@irina-herciu
Copy link
Contributor Author

I'm getting the following DDB integ test failure. Can you take a look?


[xUnit.net 00:00:06.34]     Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestContext [FAIL]
--
3720 | Failed Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestContext [3 s]
3721 | Error Message:
3722 | System.ArgumentNullException : Value cannot be null. (Parameter 'target')
3723 | Stack Trace:
3724 | at Amazon.Util.CircularReferenceTracking.Track(Object target) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Core\Amazon.Util\ReferenceTracking.cs:line 104
3725 | at Amazon.DynamoDBv2.DataModel.DynamoDBFlatConfig.OperationState.Track(Object target) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\Configs.cs:line 621
3726 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.PopulateInstance(ItemStorage storage, Object instance, DynamoDBFlatConfig flatConfig, StorageConfig storageConfig) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\ContextInternal.cs:line 443
3727 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveHelperAsync(Type valueType, Object value, DynamoDBFlatConfig flatConfig, CancellationToken cancellationToken) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\Context.cs:line 467
3728 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveHelperAsync[T](T value, DynamoDBFlatConfig flatConfig, CancellationToken cancellationToken) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\Context.cs:line 417
3729 | at Amazon.DynamoDBv2.DataModel.DynamoDBContext.SaveAsync[T](T value, CancellationToken cancellationToken) in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\src\Services\DynamoDBv2\Custom\DataModel\_async\Context.Async.cs:line 40
3730 | at Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestHashObjects() in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\test\NetStandard\IntegrationTests\IntegrationTests\DynamoDB\DataModelTests.cs:line 508
3731 | at Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestHashObjects() in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\test\NetStandard\IntegrationTests\IntegrationTests\DynamoDB\DataModelTests.cs:line 524
3732 | at Amazon.DNXCore.IntegrationTests.DynamoDB.DynamoDBTests.TestContext() in C:\codebuild\tmp\output\src2690751361\src\aws-sdk-net\sdk\test\NetStandard\IntegrationTests\IntegrationTests\DynamoDB\DataModelTests.cs:line 42
3733 | --- End of stack trace from previous location ---
3734 | Results File: C:\codebuild\tmp\output\src3842137447\src/aws-sdk-net/sdk\test\NetStandard\IntegrationTests\bin\Release\net8.0/TestResults\ContainerAdministrator_6997AF1862EC_2025-05-21_17_10_42.trx
3735 |  
3736 | Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: < 1 ms - IntegrationTests.dll (net8.0

fixed the failing test, can you please retrigger the internal build?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants